Fix missing copyright year updates and add skill checklist reminders#2982
Fix missing copyright year updates and add skill checklist reminders#2982
Conversation
…tTagging, DeleteBucketTagging) Co-authored-by: afranken <763000+afranken@users.noreply.github.com>
…ketStore Co-authored-by: afranken <763000+afranken@users.noreply.github.com>
… implemented in README Co-authored-by: afranken <763000+afranken@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds missing bucket-level tagging support to S3Mock, aligning bucket APIs with the already-supported object tagging operations and exposing the corresponding AWS S3 endpoints.
Changes:
- Persist bucket tagging in
BucketMetadataand addBucketStore.storeBucketTagging()to write it to disk. - Add
BucketService+BucketControllersupport forGetBucketTagging,PutBucketTagging, andDeleteBucketTagging(including routing exclusions to avoid Spring MVC ambiguity). - Add unit/integration test coverage and update README/CHANGELOG to document the newly supported operations.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| server/src/test/resources/com/adobe/testing/s3mock/store/BucketMetadataTest_testSerialization.json | Updates expected serialized bucket metadata JSON to include nullable tagging. |
| server/src/test/kotlin/com/adobe/testing/s3mock/store/BucketStoreTest.kt | Adds store-level test verifying bucket tagging persistence and clearing. |
| server/src/test/kotlin/com/adobe/testing/s3mock/service/BucketServiceTest.kt | Adds service-level tests for set/get/delete bucket tagging and updates metadata helper for new field. |
| server/src/test/kotlin/com/adobe/testing/s3mock/controller/BucketControllerTest.kt | Adds controller tests for GET/PUT/DELETE ?tagging bucket endpoints. |
| server/src/main/kotlin/com/adobe/testing/s3mock/store/BucketStore.kt | Adds storeBucketTagging() method to persist tagging in metadata. |
| server/src/main/kotlin/com/adobe/testing/s3mock/store/BucketMetadata.kt | Adds nullable tagging: List<Tag>? field and withTagging() copy helper. |
| server/src/main/kotlin/com/adobe/testing/s3mock/service/BucketService.kt | Implements bucket tagging operations in the service layer. |
| server/src/main/kotlin/com/adobe/testing/s3mock/controller/BucketController.kt | Adds bucket tagging routes and NOT_TAGGING exclusions to avoid request mapping ambiguity. |
| integration-tests/src/test/kotlin/com/adobe/testing/s3mock/its/BucketTaggingIT.kt | Adds AWS SDK v2 integration tests for bucket tagging lifecycle. |
| README.md | Marks bucket tagging operations as supported in the operations table. |
| CHANGELOG.md | Documents bucket tagging feature addition under 5.0.0. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| bucketStore.storeBucketTagging(bucketMetadata, tagging.tagSet.tags) | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
setBucketTagging() persists incoming tags without validating them (e.g., max tag count, duplicate keys, disallowed aws: prefix, key/value length/charset). Object tagging already enforces these constraints via ObjectService.verifyObjectTags() and throws S3Exception.INVALID_TAG; bucket tagging should apply the same rules to match S3 behavior and keep bucket/object tagging consistent (and add/adjust unit tests accordingly).
| bucketStore.storeBucketTagging(bucketMetadata, tagging.tagSet.tags) | |
| } | |
| /** | |
| val tags = tagging.tagSet.tags | |
| verifyBucketTags(tags) | |
| bucketStore.storeBucketTagging(bucketMetadata, tags) | |
| } | |
| /** | |
| * Validates bucket tags to match S3 constraints and object-tagging behavior. | |
| * Throws [S3Exception.INVALID_TAG] if any constraint is violated. | |
| */ | |
| private fun verifyBucketTags(tags: List<Tag>) { | |
| // S3 allows up to 50 tags per resource. | |
| if (tags.size > 50) { | |
| throw S3Exception.INVALID_TAG | |
| } | |
| val seenKeys = HashSet<String>() | |
| for (tag in tags) { | |
| val key = tag.key | |
| val value = tag.value | |
| // Key must be 1–128 characters. | |
| if (key.isEmpty() || key.length > 128) { | |
| throw S3Exception.INVALID_TAG | |
| } | |
| // Key must not start with the reserved "aws:" prefix (case-insensitive). | |
| if (key.startsWith("aws:", ignoreCase = true)) { | |
| throw S3Exception.INVALID_TAG | |
| } | |
| // Keys must be unique within the tag set. | |
| if (!seenKeys.add(key)) { | |
| throw S3Exception.INVALID_TAG | |
| } | |
| // Value must be at most 256 characters. | |
| if (value.length > 256) { | |
| throw S3Exception.INVALID_TAG | |
| } | |
| // Disallow ISO control characters in keys or values. | |
| if (key.any { Character.isISOControl(it) } || | |
| value.any { Character.isISOControl(it) } | |
| ) { | |
| throw S3Exception.INVALID_TAG | |
| } | |
| } | |
| } | |
| /** |
Co-authored-by: afranken <763000+afranken@users.noreply.github.com>
… skill checklists Co-authored-by: afranken <763000+afranken@users.noreply.github.com>
The initial bucket tagging implementation left four modified test files with stale
2017-2025copyright headers. The root cause: theimplement,test, andrefactorskill checklists had no explicit step for updating copyright years, so agents working through those checklists would miss the requirement even though it exists inAGENTS.md.Changes
Copyright headers — bumped
2017-2025→2017-2026in four test files modified by the bucket tagging implementation:BucketControllerTest.ktBucketServiceTest.ktBucketStoreTest.ktBucketMetadataTest.ktSkill checklists — added an explicit copyright year update step to the
## Checklistsection of all three code-touching skills:.claude/skills/implement/SKILL.md.claude/skills/test/SKILL.md.claude/skills/refactor/SKILL.mdThe new checklist item reads:
This makes the requirement actionable at the point of use rather than relying on agents recalling the
AGENTS.mdDO section.💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.